Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt the upcoming feature SE-0409 (access-level modifiers on import declarations) #1183

Merged
merged 2 commits into from
Mar 31, 2025

Conversation

d-ronnqvist
Copy link
Contributor

@d-ronnqvist d-ronnqvist commented Mar 21, 2025

Bug/issue #, if applicable:

Summary

This opts in to the upcoming feature SE-0409 (access-level modifiers on import declarations).

By making imports internal by default we make it explicit when imported dependencies are visible to library clients.

By incrementally adopting certain features beforehand we can make the future changes for updates to future languages versions smaller.

Dependencies

None

Testing

Nothing in particular. This isn't a user-facing change.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • [] Added tests
  • Ran the ./bin/test script and it succeeded
  • [ ] Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@Kyle-Ye Kyle-Ye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general. But I hope we can add "-werror" on the CI pipeline to catch up any future misuse here.

Otherwise, I'm worried this change will make a lot of warnings as we evolve the codebase.

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

LGTM in general. But I hope we can add "-werror" on the CI pipeline to catch up any future misuse here.

Otherwise, I'm worried this change will make a lot of warnings as we evolve the codebase.

So far people have for the most part been good about not introducing code warnings. Every now and then someone—myself included—misses a warning. This can for example happen when updating a PR with the latest changes from main. However, these added warnings are generally fixed fairly quickly.

That said, the CI includes some concurrency warnings that most people don't see in their local builds. It's possible that we'd be able to treat warnings as errors in the future, but right now it would be too disruptive since we haven't addressed those warnings yet. Some of those are false positives where there's another synchronization mechanism in play and some require updating dependencies to versions that we're currently unable to adopt.

@d-ronnqvist d-ronnqvist merged commit 75fa7d6 into swiftlang:main Mar 31, 2025
2 checks passed
@d-ronnqvist d-ronnqvist deleted the adopt-SE-0409-internal-import branch March 31, 2025 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants